Use robot-reported RTDE field types and improved memory copy performance - #560

include/ur_client_library/rtde/data_package.h:124

• std::string is not one of the RTDE protocol types, but retaining it here lets setData() change an untyped field from monostate to std::string. Such a package then passes isTyped() and serializePackage() emits a malformed variable-length payload, while getDataType() reports no type. Restrict untyped fields to the alternatives represented by DataType.

  using _rtde_type_variant = std::variant<std::monostate, bool, uint8_t, uint32_t, uint64_t, int32_t, double,
                                          vector3d_t, vector6d_t, vector6int32_t, vector6uint32_t, std::string>;

Previously missed (1) — in code that hasn't changed since the last review.

tests/test_rtde_allocations.cpp:52

• This thread-local design excludes not only the fake server but also the client's background-reader and writer threads. Therefore background_receive_does_not_allocate does not observe the thread that parses received packages, and sending_input_data_does_not_allocate does not observe the thread that serializes and writes them; both tests can pass with allocations in the paths their names claim to cover. Instrument all client-owned threads while excluding only the server thread.

// Counting is per-thread: the fake server and, in the background-read case, the client's read
// thread run in the same process, and their allocations are none of this test's business.
thread_local std::size_t g_allocation_count = 0;
thread_local bool g_count_allocations = false;

src/rtde/rtde_parser.cpp:161

• A caller can make a preallocated package “typed” with setData() before the first blocking receive, so this check does not prove that its types came from the robot. For example, a timestamp-only package set as uint64_t skips initEmpty(recipe_types_) and parses the robot's DOUBLE bytes as an integer while reporting success. Validate both the recipe and every existing field type against recipe_/recipe_types_, retyping or replacing packages that do not match.

      if (!data_package->isTyped())
      {
        // A package built from a recipe alone doesn't know its field types yet. Applying the ones
        // the robot reported doesn't allocate, so this happens right here rather than by handing
        // the caller a replacement package.

tests/test_rtde_data_package.cpp:460

• This test does not measure allocations, so an allocation introduced inside initEmpty(types) would still pass despite the test name and the PR's real-time guarantee. Surround this call with the allocation counter (or move this case into the allocation-test binary) and assert that its count remains zero.

  package.initEmpty({ "DOUBLE", "VECTOR6D" });

Previously missed (1) — in code that hasn't changed since the last review.

tests/test_rtde_allocations.cpp:277

• The counter is thread-local, while sendPackage() only queues data and RTDEWriter::run() performs serialization and socket writing on its own thread. Consequently this measured block cannot see allocations in the actual asynchronous send path, so sending_input_data_does_not_allocate can pass despite a send-thread regression. Add writer-thread instrumentation or a same-thread serialization allocation test.

  {
    AllocationCounter counter;
    for (int i = 0; i < g_MEASURED_CYCLES; ++i)
    {
      all_sent &= input_pkg.setData("speed_slider_fraction", 0.5);
      all_sent &= client_->getWriter().sendPackage(input_pkg);
    }

src/rtde/rtde_parser.cpp:157

• isTyped() also becomes true when the caller has populated every field with setData(), so it does not prove these are the negotiated types. A preallocated one-field timestamp package set as uint64_t, for example, skips initEmpty(recipe_types_) and parses the robot's DOUBLE bytes as UINT64. This path also never verifies field names, so an untyped same-length package with a different recipe is typed by position. Validate the recipe and always reapply the acknowledged types before parsing; replace the package only when its recipe differs.

      DataPackage* data_package = dynamic_cast<DataPackage*>(result.get());
      data_package->setProtocolVersion(protocol_version_);
      if (!data_package->isTyped())

tests/test_rtde_data_package.cpp:460

• This test never observes allocations: it only verifies that the package remains usable. The counters in test_rtde_allocations.cpp start after RTDEClient::init() and warmup, so an allocation added to initEmpty(types) would pass the suite even though no-allocation in-place typing is a central guarantee. Measure this call while allocation counting is active.

TEST(rtde_data_package, applying_types_does_not_reallocate)
{
  std::vector<std::string> recipe{ "timestamp", "actual_q" };
  test::TestableDataPackage package(recipe);

  double timestamp = 0.0;
  ASSERT_FALSE(package.getData("timestamp", timestamp));

  package.initEmpty({ "DOUBLE", "VECTOR6D" });

Previously missed (2) — in code that hasn't changed since the last review.

tests/test_rtde_allocations.cpp:184

• This guard can pass while the allocation tests miss the allocations they are intended to detect. As the comment above notes, some libstdc++/musl std::allocator implementations call malloc directly; invoking ::operator new here proves only that this replacement works, while vector/string growth in the measured RTDE paths can bypass it and leave the count at zero. Validate the counter with a representative standard-container allocation and either intercept that platform's allocation path or fail/skip when it cannot be observed.

// Guards the tests below: if the counter stopped seeing allocations, they would pass vacuously.
// Call operator new directly rather than writing `new int`: a new-expression may be omitted even
// when the pointer escapes, which is what Alpine's gcc 15 does at -O2. Allocate with operator new
// rather than a container: on some libstdc++ / musl builds std::allocator uses malloc and would
// never hit the replaced operator new that the RTDE tests count.
TEST(AllocationCounterTest, counts_allocations)
{
  std::size_t allocations = 0;
  {
    AllocationCounter counter;
    g_allocation_sink = ::operator new(sizeof(int));

src/rtde/data_package.cpp:212

• getDataType() does not necessarily report a robot-acknowledged type as the new API promises. On an application-created input package, setData() changes the variant from monostate to the caller's type, so this function then returns that inferred type—even when RTDEWriter::sendPackage() later rejects it because the robot reported a different type. Consumers therefore cannot tell whether this result is authoritative. Track the acknowledged type separately from the value/inferred type, or explicitly expose this as the stored value type and provide the robot-reported type through the client/writer API.

std::optional<rtde_interface::DataType> rtde_interface::DataPackage::getDataType(const std::string_view name) const
{
  const auto it =
      std::find_if(data_.begin(), data_.end(), [&name](const std::pair<std::string, _rtde_type_variant>& element) {
        return element.first == name;
      });
  if (it == data_.end())
  {
    return std::nullopt;
  }
  return typeOf(it->second);

• Files reviewed: 32/32 changed files
• Comments generated: 3
• Review effort level: Balanced

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

examples/rtde_writer.cpp:93

• Correct the typo in this user-facing example comment: “may” should be “many.”
  tests/test_rtde_client_reconnect.cpp:320
• These two point-in-time state checks do not establish that reconnecting stopped. During each retry the client repeatedly returns to UNINITIALIZED, so an implementation that retries forever can satisfy both assertions. Wait for the expected request count and then verify requestedProtocolVersions() remains unchanged (or expose a completion signal) to cover exhaustion rather than an incidental state between attempts.

• Files reviewed: 32/32 changed files
• Comments generated: 3
• Review effort level: Balanced

• Files reviewed: 32/32 changed files
• Comments generated: 4
• Review effort level: Balanced